feat(chalice): Add span streaming support to Chalice integration#6503
feat(chalice): Add span streaming support to Chalice integration#6503ericapisani wants to merge 3 commits into
Conversation
Add span streaming support behind the trace_lifecycle=stream experiment flag. When enabled, start a segment span with Lambda/FaaS attributes for both HTTP view functions and event source handlers instead of setting a transaction name on the scope. Also adds aws_lambda to CLOUD_PLATFORM constants. Fixes PY-2312 Fixes #6010
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 208cf35. Configure here.
| invoked_arn = aws_context.invoked_function_arn | ||
| split_invoked_arn = invoked_arn.split(":") | ||
| aws_region = split_invoked_arn[3] if len(split_invoked_arn) > 3 else "unknown" |
There was a problem hiding this comment.
Although AWS sets this value and it's unlikely to be malformed, these extra guards were added to ensure that if it were, we don't crash the user's application
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Codecov Results 📊✅ 88454 passed | ⏭️ 6022 skipped | Total: 94476 | Pass Rate: 93.63% | Execution Time: 293m 26s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 98.48%. Project has 2481 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 89.38% 89.37% -0.01%
==========================================
Files 192 192 —
Lines 23285 23333 +48
Branches 8002 8012 +10
==========================================
+ Hits 20811 20852 +41
- Misses 2474 2481 +7
- Partials 1309 1310 +1Generated by Codecov Action |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| try: | ||
| return ChaliceEventSourceHandler.__call__(self, event, context) | ||
| except Exception: | ||
| exc_info = sys.exc_info() | ||
| event, hint = event_from_exception( | ||
| exc_info, | ||
| client_options=client.options, | ||
| mechanism={"type": "chalice", "handled": False}, | ||
|
|
||
| if has_span_streaming_enabled(client.options): | ||
| span = sentry_sdk.traces.start_span( | ||
| name=context.function_name, | ||
| parent_span=None, | ||
| attributes=_get_lambda_span_attributes(context), | ||
| ) | ||
| sentry_sdk.capture_event(event, hint=hint) | ||
| client.flush() | ||
| reraise(*exc_info) |
There was a problem hiding this comment.
I don't think we need to do anything here, just leave as is -- this code doesn't do any tracing so we don't need to change it
The set of spans we're creating should be the same as in the legacy path, we don't want any new spans when streaming
| span = sentry_sdk.traces.start_span( | ||
| name=aws_context.function_name, | ||
| parent_span=None, | ||
| attributes={ | ||
| **_get_lambda_span_attributes(aws_context), | ||
| **header_attrs, | ||
| **additional_attrs, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
This is also a new span compared to the legacy path -- please remove
As I understand, this integration is built on top of the AWS Lambda integration similar to e.g. Flask and WSGI? In that case, the pattern is usually that the parent integration creates the transaction/segment encompassing the whole request-response cycle, and the child integration then augments it with info specific to the child integration (e.g. better transaction name and source). The child integration may also create its own spans (e.g. for middlewares, DB queries etc.), but it shouldn't double-instrument the same thing as the top level segment
All of this is to say -- if we don't explicitly create a span in the legacy path, we shouldn't do it in span streaming either
There was a problem hiding this comment.
In this integration specifically, we seem to just want to augment whatever transaction is currently running with the same data as in the AWS Lambda integration.
The question for me is, does this integration even make sense standalone? Because if not, then we don't need to do any sort of post-enrichment here, as it'll already have happened in the AWS Lambda integration. Otherwise, we need to duplicate the enriching logic, but apply it to the current segment instead of creating a new span.
2 participants
Add span streaming support behind the trace_lifecycle=stream experiment flag.
When enabled, start a segment span with Lambda/FaaS attributes for both
HTTP view functions and event source handlers instead of setting a
transaction name on the scope.
Also adds aws_lambda to CLOUD_PLATFORM constants.
Contains some duplication with #6498 since it's an AWS lambda framework.
Fixes PY-2312
Fixes #6010